fix(build): add GCC 16 std::nullptr_t compatibility workaround#303
fix(build): add GCC 16 std::nullptr_t compatibility workaround#303Vikyek wants to merge 1 commit into
Conversation
GCC 16.1.1 no longer exposes nullptr_t in the global namespace. Electron 42.1.0's V8 headers (v8-internal.h and others) use it unqualified, which causes compilation failures of native modules (like keytar, cpu-features, etc.) during the npm/electron-rebuild stage when building on systems with GCC 16+. This change wraps the C++ compiler (CXX) to force-include a local header containing 'using std::nullptr_t;' to pull nullptr_t back into the global namespace, but only if the C++ compiler actually requires this workaround. This ensures compatibility on newer toolchains without modifying Electron's headers directly.
There was a problem hiding this comment.
Thanks for the GCC 16 compatibility fix. I found a blocker in the detection guard that should be fixed before this lands.
Finding:
scripts/lib/native-modules.sh:176: the probe checksnullptr_twithout including<cstddef>. On the current GCC 15 toolchain this already fails (nullptr_tis defined in<cstddef>), while#include <cstddef> nullptr_t x = nullptr;passes. That means the wrapper is applied on toolchains that do not need the GCC 16 workaround, so this path always replacesCXXduring native module rebuilds instead of only doing so when Electron/V8 headers need it. Please make the probe mirror the actual failure mode, e.g. include<cstddef>before checking unqualifiednullptr_t, and add a smoke/regression test proving the wrapper is not installed on a compiler where the included probe succeeds.
Non-blocking follow-up: the wrapper path also assumes CXX is a single executable name/path, so it would not preserve common wrappers like CXX="ccache g++". That is existing-adjacent shell complexity, but if this code is touched it would be good to make the failure mode explicit.
Validation I ran:
bash -n scripts/lib/native-modules.sh tests/scripts_smoke.shgit diff --check origin/main...HEAD- local compiler probe with and without
#include <cstddef>
|
Thanks again for the GCC 16 nullptr_t compatibility report and initial fix. I landed a scoped version in #310 that keeps the workaround gated behind a -based probe, applies it to both the regular native-module rebuild path and the Nix native-modules build, and adds smoke coverage for both the no-wrapper and wrapper-needed cases. Closing this PR as superseded by #310. |
2 participants
GCC 16.1.1 no longer exposes
nullptr_tin the global namespace. Electron 42.1.0's V8 headers (v8-internal.hand others) use it unqualified, which causes compilation failures of native modules (likekeytar,cpu-features, etc.) during the npm/electron-rebuild stage when building on systems with GCC 16+.This change wraps the C++ compiler (
CXX) to force-include a local header containingusing std::nullptr_t;to pullnullptr_tback into the global namespace, but only if the C++ compiler actually requires this workaround. This ensures compatibility on newer toolchains without modifying Electron's headers directly.